Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

[scheduler, mtsource] Initial HA support for MT Kafka source #587

Merged
merged 6 commits into from
May 12, 2021
Merged

[scheduler, mtsource] Initial HA support for MT Kafka source #587

merged 6 commits into from
May 12, 2021

Conversation

aavarghese
Copy link
Contributor

@aavarghese aavarghese commented Apr 29, 2021 •
edited
Loading

Fixes #463

Depends on knative/eventing#5360

Proposed Changes

  • Controller has new env var to control "scheduler policy type",
  • Stateful set adapter has new anti-affinity rules for pods to be placed on new nodes in new zone topology,
  • "scheduler policy type" is an enum with two values currently MaxFillup and EvenSpread. For EvenSpread, scheduler places vreplicas for HA by spreading replicas equally across zones. Within a zone, vreplicas are filled up in each pod upto capacity. If MaxFillup strategy is chosen, scheduler places vreplicas in existing pods and fills them up before creating new pods (zones aren't considered)
  • Placement type in KafkaSource status has a new type for ZoneName when EvenSpread scheduling
  • HA controls are also shared with autoscaler, and state accessor
  • State accessor computes the number of zones in cluster and stores a map of node name to zone info

Release Note

@aavarghese aavarghese requested review from a team as code owners April 29, 2021 18:04
@aavarghese aavarghese requested a review from a team April 29, 2021 18:04
@aavarghese aavarghese requested review from a team as code owners April 29, 2021 18:04
@aavarghese aavarghese requested a review from a user April 29, 2021 18:04
@knative-prow-robot knative-prow-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 29, 2021
@google-cla google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Apr 29, 2021
@aavarghese aavarghese changed the title [WIP] Initial HA support for Kafka source [WIP] Initial HA support for MT Kafka source Apr 29, 2021
@knative-prow-robot knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 29, 2021
@codecov
Copy link

codecov bot commented Apr 29, 2021 •
edited
Loading

Codecov Report

Merging #587 (781f66c) into main (5350b3b) will increase coverage by 0.51%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #587      +/-   ##
==========================================
+ Coverage   72.29%   72.81%   +0.51%     
==========================================
  Files         140      140              
  Lines        5675     5811     +136     
==========================================
+ Hits         4103     4231     +128     
- Misses       1357     1358       +1     
- Partials      215      222       +7
Impacted Files Coverage Δ
pkg/common/scheduler/statefulset/scheduler.go 81.86% <84.80%> (+15.19%) ⬆️
pkg/common/scheduler/statefulset/state.go 88.00% <90.00%> (+2.70%) ⬆️
pkg/common/scheduler/statefulset/autoscaler.go 73.58% <100.00%> (+0.50%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5350b3b...781f66c. Read the comment docs.

@knative-prow-robot knative-prow-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 30, 2021
@knative-prow-robot knative-prow-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 6, 2021
@knative-prow-robot knative-prow-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 6, 2021
@aavarghese aavarghese force-pushed the issue#463 branch 4 times, most recently from 70145ac to 6d8e1bf Compare May 10, 2021 21:49
aavarghese added 5 commits May 11, 2021 17:59
@aavarghese
Even spread for HA implementation
96a28e1 
Signed-off-by: Ansu Varghese <avarghese@us.ibm.com>
@aavarghese
Ignore nodes that don't have a zone label in state
75ee2b2 
Signed-off-by: Ansu Varghese <avarghese@us.ibm.com>
@aavarghese
Fix for not removing enough replicas
22d95fb 
Signed-off-by: Ansu Varghese <avarghese@us.ibm.com>
@aavarghese
Changing SchedulerPolicyType from int to string
bbd2170 
Signed-off-by: Ansu Varghese <avarghese@us.ibm.com>
@aavarghese
Adding node lister and pod lister to get info from cache to compute s…
ca43d71 
…pread etc

Signed-off-by: Ansu Varghese <avarghese@us.ibm.com>
@aavarghese aavarghese changed the title Initial HA support for MT Kafka source [scheduler, mtsource] Initial HA support for MT Kafka source May 11, 2021
@aavarghese
Reverting scheduler policy type to default strategy
781f66c 
Signed-off-by: Ansu Varghese <avarghese@us.ibm.com>
@knative-metrics-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-knative-sandbox-eventing-kafka-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/common/scheduler/statefulset/autoscaler.go 76.9% 77.5% 0.6
pkg/common/scheduler/statefulset/scheduler.go 70.9% 86.9% 16.0
pkg/common/scheduler/statefulset/state.go 91.4% 93.9% 2.4

@lionelvillard
Copy link
Contributor

Let's get this in so we can move forward with the issues raised during the review.

Thanks @aavarghese this is amazing work!

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label May 12, 2021
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lionelvillard

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 12, 2021
@knative-prow-robot knative-prow-robot merged commit 93bdb1c into knative-extensions:main May 12, 2021
@aavarghese aavarghese mentioned this pull request Jun 14, 2021
Closed
@pierDipi pierDipi mentioned this pull request Oct 4, 2021
Closed
matzew pushed a commit to matzew/eventing-kafka that referenced this pull request Feb 24, 2022
@openshift-cherrypick-robot @knative-prow-robot @mgencur
Prevent excessive wait time in KafkaSender for upgrade tests (knative…
6848d14 
…-extensions#1107) (knative-extensions#587)

Co-authored-by: Martin Gencur <mgencur@redhat.com>

Co-authored-by: Knative Prow Robot <knative-prow-robot@google.com>
Co-authored-by: Martin Gencur <mgencur@redhat.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@matzew matzew matzew left review comments

@lionelvillard lionelvillard lionelvillard approved these changes

Assignees

@lionelvillard lionelvillard

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[scheduler,mtsource] support for HA
5 participants
@aavarghese @knative-metrics-robot @lionelvillard @knative-prow-robot @matzew